Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: gradually reduce null-check errors #3094

Merged
merged 14 commits into from Feb 14, 2023
Merged

Conversation

gastonfournier
Copy link
Contributor

@gastonfournier gastonfournier commented Feb 13, 2023

About the changes

In order to move us towards enabling strictNullChecks we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues.

This new workflow:

  1. Checks out the current branch and main into 2 different folders
  2. Uses the same script gradual-strict-null-checks.sh (from the current branch) against each folder in parallel to count the number of errors if strictNullChecks was enabled
  3. If the number of potential errors in the current branch is higher than the number of potential errors in main it fails

As an example, a new issue was introduced in this PR (and then reverted), so we can test the build failure:
https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10

Discussion points

This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears

In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with // @ts-ignore: Object is possibly 'null'. although might defeat the purpose of this workflow

@vercel
Copy link

vercel bot commented Feb 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 2:49PM (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 2:49PM (UTC)

@gastonfournier gastonfournier changed the title chore: try to reduce null-checks against main chore: gradually reduce null-check errors Feb 13, 2023
@gastonfournier gastonfournier self-assigned this Feb 13, 2023
@gastonfournier gastonfournier marked this pull request as ready for review February 13, 2023 13:47
Copy link
Contributor

@chriswk chriswk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea. I'm a +1 on this

Copy link
Contributor

@chriswk chriswk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an approve as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants